Fix (target start): curveadm target start timeout #364
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@zztaki We recommend modifying the commit message Fix(target start): ... and It will greatly improve convenience and readability. |
caoxianfei1
reviewed
Nov 24, 2023
internal/task/task/bs/start_tgtd.go
Outdated
| @@ -127,7 +127,7 @@ func NewStartTargetDaemonTask(curveadm *cli.CurveAdm, cc *configure.ClientConfig | |||
| ExecOptions: curveadm.ExecOptions(), | |||
| }) | |||
| t.AddStep(&step.ContainerExec{ | |||
| Command: "tgtd -f &", | |||
| Command: "-d tgtd -f &", | |||
There was a problem hiding this comment.
Maybe modify the step of ContainerExec it better?
ContainerExec{
Command: "tgtd -f",
Detached: true,
...
}There was a problem hiding this comment.
Oh, it's attractive! I didn't notice there was this option
There was a problem hiding this comment.
But you need to add it, now it doesn't support --detach option
zztaki
changed the title
Signed-off-by: zztaki <[email protected]>
zztaki
force-pushed
the
fix-target-start-timeout
branch
from
28bc0b9
to
ed38480
Compare
|
LGTM! |
caoxianfei1
approved these changes
Nov 24, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to this issue.
In curve tgt deployment, we start the tgtd daemon on the specified host using
curveadm target start --host target-host -c client.yaml. This command will timeout but in fact, the tgtd has been started.Looking through these codes, I found this command is mainly executed by
sudo docker exec <ContianerId> tgtd -f &on the remote host and we need to wait its returns (combined stdout and stderr) to end with no error. But sincesudo docker exec <ContianerId> tgtd -f &executes in non-detached mode, it will occupy its stdout, which causes the callback function can't return and then timeout.So, I try to add
-dparameter to ContainerExec, which makes tgtd running in detached mode correctly.